Skip to content

Conversation

ArekBalysNordic
Copy link
Contributor

The CODEOWNERS file overrides all Kconfig, CMakeLists and .cmake file patterns, and due to that, ncs-matter is not notified about these file types in Matter locations.

To fix it, assign both nrfconnect/ncs-co-build-system nrfconnect/ncs-matter to all Matter-related Kconfig, CMakeLists and .cmake files.

@ArekBalysNordic ArekBalysNordic requested a review from a team as a code owner August 25, 2025 13:06
@NordicBuilder
Copy link
Contributor

NordicBuilder commented Aug 25, 2025

CI Information

To view the history of this post, clich the 'edited' button above
Build number: 5

Inputs:

Sources:

sdk-nrf: PR head: 74d80367c01a85f873ba3acadd2d4e5536fc088b

more details

sdk-nrf:

PR head: 74d80367c01a85f873ba3acadd2d4e5536fc088b
merge base: 924dce5e3e09d99831d0411e83c44fe06b796022
target head (main): 1f2ca0089a3bb9e21bace02cd367641733e18cc8
Diff

Github labels

Enabled Name Description
ci-disabled Disable the ci execution
ci-all-test Run all of ci, no test spec filtering will be done
ci-force-downstream Force execution of downstream even if twister fails
ci-run-twister Force run twister
ci-run-zephyr-twister Force run zephyr twister
List of changed files detected by CI (1)
CODEOWNERS

Outputs:

Toolchain

Version:
Build docker image:

Test Spec & Results: ✅ Success; ❌ Failure; 🟠 Queued; 🟡 Progress; ◻️ Skipped; ⚠️ Quarantine

  • ◻️ Toolchain
  • ◻️ Build twister
  • ◻️ Integration tests
Disabled integration tests
    • test-fw-nrfconnect-nrf_lrcs_mosh
    • test-fw-nrfconnect-nrf_lrcs_positioning
    • desktop52_verification
    • doc-internal
    • test_ble_nrf_config
    • test-fw-nrfconnect-apps
    • test-fw-nrfconnect-ble_samples
    • test-fw-nrfconnect-chip
    • test-fw-nrfconnect-fem
    • test-fw-nrfconnect-nfc
    • test-fw-nrfconnect-nrf-iot_cloud
    • test-fw-nrfconnect-nrf-iot_libmodem-nrf
    • test-fw-nrfconnect-nrf-iot_lwm2m
    • test-fw-nrfconnect-nrf-iot_samples
    • test-fw-nrfconnect-nrf-iot_serial_lte_modem
    • test-fw-nrfconnect-nrf-iot_thingy91
    • test-fw-nrfconnect-nrf-iot_zephyr_lwm2m
    • test-fw-nrfconnect-nrf_crypto
    • test-fw-nrfconnect-ps-main
    • test-fw-nrfconnect-rpc
    • test-fw-nrfconnect-rs
    • test-fw-nrfconnect-tfm
    • test-fw-nrfconnect-thread-main
    • test-low-level
    • test-sdk-audio
    • test-sdk-dfu
    • test-sdk-find-my
    • test-sdk-mcuboot
    • test-sdk-pmic-samples
    • test-sdk-wifi
    • test-secdom-samples-public

Note: This message is automatically posted and updated by the CI

@ArekBalysNordic ArekBalysNordic force-pushed the matter_codeowners branch 2 times, most recently from edf4edb to 0b81fe1 Compare August 25, 2025 13:14
The CODEOWNERS file overrides Kconfig, CMakeLists and .cmake
file patterns, and due to that, ncs-matter is not notified about
these file types in Matter locations.

To fix it, assign both nrfconnect/ncs-co-build-system
nrfconnect/ncs-matter to all Matter-related Kconfig, CMakeLists\
and .cmake files.

Signed-off-by: Arkadiusz Balys <[email protected]>
@ArekBalysNordic ArekBalysNordic changed the title Add ncs-matter to Matter-specific files CODEOWNERS: Add ncs-matter to Matter-specific files Aug 25, 2025
The CODEOWNERS file overrides all Kconfig, CMakeLists and .cmake
file patterns, and due to that, ncs-thread is not notified about
these file types in Thread-specific locations.

Signed-off-by: Arkadiusz Balys <[email protected]>
Copy link
Contributor

@nordicjm nordicjm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs discussion with @carlescufi from what I remember this means that a review is needed from one of the listed groups, and that would be wrong

@nordicjm
Copy link
Contributor

Yes as per https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/customizing-your-repository/about-code-owners?azure-portal=true

# In this example, any change inside the `/scripts` directory
# will require approval from @doctocat or @octocat.
/scripts/ @doctocat @octocat

this is not acceptable

@ArekBalysNordic
Copy link
Contributor Author

ArekBalysNordic commented Aug 26, 2025

needs discussion with @carlescufi from what I remember this means that a review is needed from one of the listed groups, and that would be wrong

Hmm, so it is even better! I know that you have a lot of work, and there are only two people in the ncs-co-build-system, so if we want to, for example, change a simple Kconfig value related to Matter/Thread, it would be great if we could avoid bothering you for review. :)

@carlescufi I believe it simplifies work for ncs-co-build-system, ncs-matter and ncs-thread :)

@nordicjm
Copy link
Contributor

needs discussion with @carlescufi from what I remember this means that a review is needed from one of the listed groups, and that would be wrong

Hmm, so it is even better! I know that you have a lot of work, and there are only two people in the ncs-co-build-system, so if we want to, for example, change a simple Kconfig value related to Matter/Thread, it would be great if we could avoid bothering you for review. :)

@carlescufi I believe it simplifies work for ncs-co-build-system, ncs-matter and ncs-thread :)

nack, results in complete abuse of features e.g. #24027

@kkasperczyk-no
Copy link
Contributor

needs discussion with @carlescufi from what I remember this means that a review is needed from one of the listed groups, and that would be wrong

Hmm, so it is even better! I know that you have a lot of work, and there are only two people in the ncs-co-build-system, so if we want to, for example, change a simple Kconfig value related to Matter/Thread, it would be great if we could avoid bothering you for review. :)
@carlescufi I believe it simplifies work for ncs-co-build-system, ncs-matter and ncs-thread :)

nack, results in complete abuse of features e.g. #24027

Yeah, the usage of snippets there was not a good fit, as probably the shape of configuration changed in between. But the PR that introduced it was approved by you, isn't it: #20072?

@ArekBalysNordic
Copy link
Contributor Author

needs discussion with @carlescufi from what I remember this means that a review is needed from one of the listed groups, and that would be wrong

Hmm, so it is even better! I know that you have a lot of work, and there are only two people in the ncs-co-build-system, so if we want to, for example, change a simple Kconfig value related to Matter/Thread, it would be great if we could avoid bothering you for review. :)
@carlescufi I believe it simplifies work for ncs-co-build-system, ncs-matter and ncs-thread :)

nack, results in complete abuse of features e.g. #24027

Why so aggressive? :( I want to help with simplifying and speeding up our work. We're not removing ncs-co-build-system from review; we want to see your opinion and suggestions, especially as mentioned in the linked PR. It is essential for us to be notified if anything changes in those files. It is important for us, and without that, we have no control. For now, we don't have a better system to track such changes, so it is one of the possible solutions. Let's wait for the @carlescufi statement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants